Skip to content

Conversation

@zztkm
Copy link
Contributor

@zztkm zztkm commented Jan 6, 2026

SimulcastRequestRid は unspecified (未指定) が含まれており、Signaling の type: connect 用になっている。
そのため、この型を RPC 機能などの rid を表す項目に利用するのは不適切であると判断し、Rid 列挙型を新たに追加してこの型を利用するように変更した。

@zztkm

This comment was marked as resolved.

@github-actions

This comment was marked as resolved.

@zztkm

This comment was marked as resolved.

@github-actions

This comment was marked as resolved.

@zztkm

This comment was marked as resolved.

@github-actions

This comment was marked as resolved.

@zztkm

This comment was marked as resolved.

@github-actions

This comment was marked as resolved.

Comment on lines +52 to +66
- RPC メソッドを表す列挙型 `RPCMethod` を追加する
- `SignalingOffer` に以下の項目を追加する
- `rpcMethods: [String]?`
- `simulcastRpcRids: [Rid]?` を追加する
- `MediaChannel``rpc` メソッドを追加する
- `MediaChannel` に以下の項目を追加する
- `rpcMethods: [RPCMethod]`
- `rpcSimulcastRids: [Rid]`
- RPC メソッドを定義するための `RPCMethodProtocol` プロトコルを追加する
- `RPCMethodProtocol` に準拠した型を追加する
- `RequestSimulcastRid`
- `RequestSpotlightRid`
- `ResetSpotlightRid`
- `PutSignalingNotifyMetadata`
- `PutSignalingNotifyMetadataItem`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここらの追加分は #291 で書けてなかった内容を追加したものです

Comment on lines +10 to +21
# build
build:
xcodebuild \
-scheme 'Sora' \
-sdk iphoneos26.1 \
-configuration Release \
-derivedDataPath build \
-destination 'generic/platform=iOS' \
clean build \
CODE_SIGNING_REQUIRED=NO \
CODE_SIGN_IDENTITY= \
PROVISIONING_PROFILE=
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あったら使うかなと思い追加しました

@zztkm
Copy link
Contributor Author

zztkm commented Jan 6, 2026

!sonnet commit 109547f で Rid 型変更が漏れていた箇所を追加対応しました。問題ないかレビューして

@github-actions
Copy link

github-actions bot commented Jan 6, 2026

Claude finished @zztkm's task in 1m 25s —— View job


レビュー完了

コミット 109547f の変更内容を確認しました。問題ありません。

確認事項

  1. RPCTypes.swift の型変更は完全

    • RequestSimulcastRidParamsrid: Stringrid: Rid への変更完了
    • RequestSpotlightRidParamsspotlightFocusRid, spotlightUnfocusRid の型変更完了
    • RequestSimulcastRidResultrid の型変更完了
    • RequestSpotlightRidResult の両 rid パラメータの型変更完了
    • すべての init メソッドのパラメータ型も一致
  2. Rid 型の実装は適切

    • Types.swift:1-42 に Rid 列挙型が正しく定義されている
    • none, r0, r1, r2 の 4 つのケースが含まれている
    • Codable プロトコルに準拠し、エンコード / デコードが適切に実装されている
  3. 他ファイルの変更も確認済み

    • MediaChannel.swift:183rpcSimulcastRids の型が [Rid] に変更済み
    • PeerChannel.swift:129, 959rpcSimulcastRids 関連の型が [Rid] に変更済み
    • RPC.swift:75, 78simulcastRpcRids パラメータ型が [Rid] に変更済み
    • Signaling.swift:482simulcastRpcRids: [Rid]? が追加済み
    • 不要になった toSimulcastRequestRids() 変換メソッドの呼び出しが削除済み
  4. テストコードが追加されている

    • RidTests.swift でエンコード / デコードのテストが追加されている
    • SignalingOfferTests.swiftsimulcastRpcRids のデコードテストが追加されている
  5. SimulcastRequestRid との使い分けが明確

    • SimulcastRequestRidtype: connectsimulcast_request_rid 用として残っている (.unspecified を含む)
    • Rid は RPC 機能と type: offersimulcast_rpc_rids 用として使われている (.unspecified を含まない)
    • この使い分けは PR の目的に合致している

型変更が漏れていた箇所 (コミット 109547f で対応済み)

コミット 109547f で以下の箇所が適切に修正されています:

  • RequestSimulcastRidParamsRequestSpotlightRidParamsEncodableCodable への変更 (デコードも可能にするため)
  • RPC パラメータと結果の型すべてが StringRid に変更されている

CHANGES.md について

66-80 行目の追加内容について、RPC 機能追加の記載は適切です。ただし、67 行目の「Rid 列挙型を追加する」という項目が追加されていますが、これは既に PR #295 のメインテーマとして最初のコミットで実装済みの内容です。この項目は重複している可能性があります。

総評

問題なし。 コミット 109547f で RPCTypes.swift の型変更が完全に対応されています。Rid 型への変換は一貫しており、変換漏れはありません。


@zztkm zztkm requested a review from t-miya January 6, 2026 07:02
@zztkm zztkm changed the title [WIP] Rid 列挙型を追加する Rid 列挙型を追加する Jan 6, 2026
Copy link
Contributor

@t-miya t-miya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@zztkm zztkm merged commit d6e3465 into develop Jan 6, 2026
3 checks passed
@zztkm zztkm deleted the feature/rid-type branch January 6, 2026 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants